fix: unify timed sheet behavior for master #565
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
app/src/main/java/to/bitkit/utils/timedsheets/sheets/AppUpdateTimedSheet.kt
Show resolved
Hide resolved
ovitrif
changed the title
|
for this case, the user now have to leave the home and return to trigger the backup sheet |
|
@jvsena42 Can I review this? |
yes, was waiting for CI, but need some updates from E2E |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Whoops... sorry, I was on previous branch |
| @@ -0,0 +1,21 @@ | |||
| package to.bitkit.utils.timedsheets | |||
There was a problem hiding this comment.
nit: we don't need another package only for this.
nit: Also, we should collectively put shared code in "shared", not utils.
nit: and lastly, I things that belong together should stay together, this global scope function is better placed in TimedSheetManager class, which should also include the TimedSheetItem definition.
|
|
||
| suspend fun shouldShow(): Boolean | ||
| suspend fun onShown() | ||
| suspend fun onDismissed() |
There was a problem hiding this comment.
nit: Name should not be past tense as per callback naming best practices.
| @@ -0,0 +1,21 @@ | |||
| package to.bitkit.utils.timedsheets | |||
There was a problem hiding this comment.
nit: package of all these should be .ui.sheets.* where it could be grouped into *sheets.timed if desired.
| object TimedSheetModule { | ||
|
|
||
| @Provides | ||
| fun provideTimedSheetManagerProvider(): (CoroutineScope) -> TimedSheetManager { |
There was a problem hiding this comment.
nit: do we really need a provider for this?!
Can't we maybe inherit BaseCoroutineScope?!
AFAIU delegation using by CoroutineScope(coroutineContext) is the recommended best practice
Close #497
Description
context: #554 (comment)
This branch brings the changes from #556 and #562 to a branch based on master
Preview
backup_and_high-balance.webm
notifications_and_quickpay.webm
critical_update.webm
internal_navigation.webm
QA Notes
Same tests from #562